Support model methods & standalone functions for models with external C++ (user_header)#1197
Open
ssp3nc3r wants to merge 1 commit into
Open
Support model methods & standalone functions for models with external C++ (user_header)#1197ssp3nc3r wants to merge 1 commit into
ssp3nc3r wants to merge 1 commit into
Conversation
…ders `compile_model_methods = TRUE` and `compile_standalone = TRUE` previously failed for any model using an external C++ `user_header`, for two reasons: 1. The standalone / model-methods C++ was generated by calling stanc directly (via an argument vector, no shell) using the same quoted `--name='foo_model'` flag that is built for the make-based executable build. With no shell to strip them, the quotes became part of the model name, producing the namespace `'foo_model'_namespace`. External C++ functions, which are defined in `foo_model_namespace`, were therefore not visible and compilation failed with "use of undeclared identifier". 2. `rcpp_source_stan()` did not `-include` the user header when compiling the model methods via `Rcpp::sourceCpp()`, unlike CmdStan's makefile which adds `-include $(USER_HEADER)`. Pass an unquoted form of the stanc options to `get_standalone_hpp()`, and force-include the user header in `rcpp_source_stan()`. With both fixes, `grad_log_prob()` on a model with a hand-written C++ likelihood matches the pure-Stan autodiff version exactly. Adds a regression test.
62ffb92 to
c4534ec
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1197 +/- ##
==========================================
+ Coverage 91.10% 91.16% +0.06%
==========================================
Files 15 15
Lines 6125 6138 +13
==========================================
+ Hits 5580 5596 +16
+ Misses 545 542 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submission Checklist
Summary
compile_model_methods = TRUE(the$log_prob()/$grad_log_prob()/$hessian()methods) andcompile_standalone = TRUE(expose_functions()) currently fail to compile for any model that uses an external C++user_header— even though the very same model samples without issue. This blocks a common workflow: validating a hand-written C++ log density / analytic gradient against the pure-Stan autodiff version viagrad_log_prob().Reproducible example
bernoulli_external.stan(declaresmake_odds, defined in the header):make_odds.hpp(in the model's namespace, per the CmdStan convention):mod$sample()works, but compiling the model methods aborts with:Root cause
Two independent problems in the standalone / model-methods build path:
Mangled namespace. The standalone & model-methods C++ is generated by calling
stancdirectly through an argument vector (no shell) inget_standalone_hpp(), reusing the same--name='foo_model'flag that is built for the make-based executable build. The make path is processed by a shell that strips the quotes; the direct call is not, so the literal quotes become part of the model name, producing the namespace'foo_model'_namespace(x39model...x39_namespaceafter C++ name-mangling). External C++ functions are defined by the user infoo_model_namespace, so they are not visible to the generated code.Header not included.
rcpp_source_stan()does not-includethe user header when compiling the methods viaRcpp::sourceCpp(), unlike CmdStan's makefile, which adds-include $(USER_HEADER).Fix
get_standalone_hpp()calls; the make path keeps the quoted form (so names/paths with spaces are still safe there).self$functions) and force-include it inrcpp_source_stan(), mirroring CmdStan's-include $(USER_HEADER).This also enables
compile_standalone = TRUE/expose_functions()for models with external C++, which sharercpp_source_stan().Verification
For a normal model and an identical model whose single likelihood term is routed through an external C++ function, with the fix
log_prob()andgrad_log_prob()match the pure-Stan autodiff version exactly:Adds a regression test in
tests/testthat/test-model-methods.Rusing the existingbernoulli_externalfixture (fulltest-model-methods.Rpasses locally on macOS / CmdStan 2.39.0).Copyright and Licensing
Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Scott Spencer
By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses: